test(typescript): expand multimodal coverage - #846
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Performance
✓ No regressions detected |
📊 Coverage gateThresholds from
✅ Gate passedNo surface regressed past the allowed threshold and the aggregate stayed above the floor. |
📐 Patch coverage gateThreshold: 80% on lines this PR touches vs
✅ Patch gate passedEvery surface whose lines were touched by this PR has patch coverage at or above the threshold. |
AbirAbbas
left a comment
AbirAbbas
left a comment
There was a problem hiding this comment.
Took this one over since it had gone quiet.
I mutation-tested the suite rather than just reading it: deliberately broke the multimodal source four ways (image MIME fallback, the response.ok failure path, case-insensitive generic MIME inference, and the extensionless-file branch) and confirmed which breaks the tests actually catch. Three of the four were caught with precise assertions, so this is genuine coverage rather than tests that mirror the implementation.
The one that got through was Audio.fromUrl's default format — flipping it from wav to flac left all 16 tests green, because every successful-download test passed an explicit format. I've pushed a commit adding that case, and verified it fails against the mutation and passes once reverted.
One note on the PR description: the full npm test run is clean here — 823/823 on your branch before my commit, and 807/807 on current main — so the resource-sensitive memory-leak failure you flagged didn't reproduce. Nothing to do, just so it's not left as an open question.
CI is green now; the checks weren't running earlier because the workflow runs were sitting behind the first-time-contributor approval gate on our side, not because of anything you did.
2 participants
Summary
Adds the requested comprehensive TypeScript SDK tests for multimodal helpers. The new suite exercises text construction, image and audio factories, real temporary-file round trips, mocked audio downloads and failures, format fallbacks, and MIME inference for images, audio, documents, and extensionless files.
Type of change
Test plan
cd sdk/typescript && npm test -- tests/multimodal_comprehensive.test.ts(16 passed)cd sdk/typescript && npm test -- tests/multimodal.test.ts tests/multimodal_comprehensive.test.ts --coverage --coverage.reportsDirectory=coverage-407(22 passed)cd sdk/typescript && npm run lintcd sdk/typescript && npm test(822/823 passed; the existing resource-sensitive memory leak test reports 31.4 MB growth against a 25 MB threshold and also fails in isolation)Test coverage
Using the unchanged
multimodal.test.tsplus the new comprehensive suite,src/ai/multimodal.tsline/statement coverage increases from 38.14% to 86.59%; branch coverage is 89.58%.Checklist
docs/CONTRIBUTING.mdanddocs/DEVELOPMENT.md.maincommits are also unsigned).Related issues / PRs
Closes #407